fix(td): harden task detail payload and timestamp handling#20
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Task Detail view by enriching the data displayed and improving its robustness. It introduces a dedicated section for scheduling details, integrates comments directly into the activity feed, and refines how timestamps are parsed and presented. These changes aim to provide a more comprehensive and reliable task overview for users, while also ensuring backend stability with new data structures. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Review - fix(td): harden task detail payload and timestamp handlingOverall this is clean, well-structured work. The tdTimestamp module is a solid extraction - the offset normalization logic is correct and the tests pin the key UTC handling contracts. A few things worth addressing before merge. Bug: defer_count appears twice in the Details tab
In When Unnecessary type cast in taskDetailLayout.tsLine 62:
hasSchedulingDetails includes created_branch
Minor: nowMs namingPer the project single-word naming convention, Minor: formatTdAbsolute has no direct test
What is working well
The |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f0c63c0fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const offsetToken = tail.match(/^(Z|z|[+-]\d{2}:?\d{2})\b/)?.[1] | ||
| const offsetMinutes = offsetToken ? parseOffsetMinutes(offsetToken) : null | ||
| const utcMs = Date.UTC(year, month, day, hour, minute, second, ms) |
There was a problem hiding this comment.
Reject invalid date parts before constructing UTC timestamp
parseTdTimestamp feeds regex captures directly into Date.UTC, which silently normalizes out-of-range components instead of failing (for example, 2026-02-30 becomes March 2). In environments where td payloads contain malformed datetime/date strings (manual DB edits, partial migrations, or corrupted rows), the modal will display a plausible but wrong time/date rather than falling back to raw text or —, which is an accuracy regression from the previous parser behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request significantly hardens the task detail view by improving timestamp handling and adding robust support for comments and other scheduling fields. The introduction of the tdTimestamp utility is a great improvement for consistency and correctness. The backend changes are also robust, gracefully handling missing database tables. I have a couple of suggestions to address a minor UI redundancy and a style guide violation in the backend.
| <span className="text-muted-foreground">Defer Count</span> | ||
| <span>{issue.defer_count}</span> |
There was a problem hiding this comment.
The Defer Count is displayed in the "Metadata" section, but it's also shown in the "Scheduling" section when its value is greater than zero. This creates redundant information in the UI. To avoid this duplication, I suggest removing it from the "Metadata" section, as "Scheduling" is a more appropriate place for this information.
| getComments(issueId: string): TdComment[] { | ||
| try { | ||
| const db = this.open(); | ||
| return db | ||
| .prepare( | ||
| 'SELECT * FROM comments WHERE issue_id = ? ORDER BY created_at ASC', | ||
| ) | ||
| .all(issueId) as TdComment[]; | ||
| } catch (error) { | ||
| logger.error(`[TdReader] Failed to get comments for ${issueId}`, error); | ||
| return []; | ||
| } | ||
| } |
There was a problem hiding this comment.
The new getComments method uses a try/catch block for error handling. While this is consistent with the rest of the file, it deviates from the repository's preferred error handling pattern.
Refactoring this to use Effect-ts would better align with the project's coding standards. This might require a broader refactoring of the TdReader service to be asynchronous and effect-based, which could be considered for future work.
References
- The style guide specifies using Effect-ts patterns for error handling instead of raw try/catch blocks. The new
getCommentsfunction uses atry/catchblock. (link)
Summary
This PR completes td-b6a5b7 (TD Task Detail View Enrichment) and closes its remaining subtask work for timestamp and comments robustness.
What changed
parent_idcreator_sessiondefer_count+HH:MM,+HHMM,Z).commentstable is missing (returns empty comments, no crash).Files
client/src/components/TaskDetailModal.tsxclient/src/components/TaskDetailModal.test.tsclient/src/lib/taskDetailLayout.tsclient/src/lib/types.tsclient/src/lib/tdTimestamp.tsclient/src/lib/tdTimestamp.test.tssrc/services/tdReader.tssrc/services/tdReader.test.tsValidation
bun run test -- client/src/lib/tdTimestamp.test.ts client/src/components/TaskDetailModal.test.ts src/services/tdReader.test.ts src/services/apiServer.test.ts37/37tests passedbun run typecheckcd client && bun run buildTD tracking
td-b6a5b7(closed)td-f7106btd-79454atd-cbb80etd-2d8945td-f28e7f